Repository navigation
RDBC-1092 Support PKCS#12 client certificates on Bun - #583
Merged
Merged
Conversation
Bun's fetch silently ignores the `tls.pfx` option (oven-sh/bun#41958, oven-sh/bun#17543), so a PFX certificate configured through authOptions was never presented and the request went out uncertified. Its node:https implementation does honour `pfx` (oven-sh/bun#14417), so route those requests there instead. BunHttpUtil installs a fetch-compatible transport backed by node:https as the request `fetcher` - the same seam conventions.customFetch uses - when a PKCS#12 archive is configured on Bun. PEM material keeps using Bun's `tls` fetch option, which works. The transport covers what the client asks of fetch: methods, headers, string/binary/FormData/stream request bodies, streamed response bodies, AbortSignal, and content-encoding decompression. It is built lazily, once per executor, and closed in dispose(). Before Bun 1.4.0 neither transport presents a client certificate (node:https ignored the certificate and the ca as well), so DocumentStore.initialize() now throws an actionable error there instead of failing per request. RavenCommand.send passes a node:stream Readable body through when the fetcher declares it accepts one, so streamed attachment uploads keep working on that path. Drops the now-obsolete console.warn from PfxCertificate.toBunTlsOptions and documents the Bun runtime, including the pre-existing Changes API gap (Bun's built-in WebSocket ignores the ws package's TLS options).
Route the response through stream.pipeline instead of res.pipe(decompressor): pipe() forwards data only, so when the connection dropped or the request was aborted mid-way through a compressed body the gunzip stream never ended or errored and the body never settled. Reproduced on both Node and Bun. Send an accept-encoding header built from what zlib can decode (gzip, deflate, br, and zstd where available) unless the caller set one. node:http sends none by default, unlike fetch, so the server never compressed and the decoding path was dead code; the executor still opts out with `identity` when conventions.useHttpDecompression is off. Decide the body-less HEAD/204/205/304 case before building a decompressor. A gunzip fed the empty input of a HEAD reply that carries content-encoding failed with an unhandled Z_BUF_ERROR. Surface the signal's AbortError on the body when an abort lands mid-way, as fetch does, instead of the bare ECONNRESET the socket teardown produces, so the executor's timeout path still recognises it. Decide requiresNodeHttpsTransport once in the RequestExecutor constructor rather than on every request. Add a PKCS#12 client-certificate leg to the Bun workflow (server 7.2): with RAVENDB_TEST_CLIENT_CERT_PATH empty, TestUtil falls back to the server's PFX as the client certificate, so the node:https transport is exercised against a real server in CI.
kalczur
requested changes
Sep 15, 2026
…terministic Comments describe current behaviour only; the rationale moved to the PR description. The connection-drop and abort tests sequence the fault after fetch() resolves instead of on a timer, and the hang guard and settle wait are gone.
kalczur
approved these changes
Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue link
https://issues.hibernatingrhinos.com/issue/RDBC-1092
Description
Bun's
fetchsilently ignores thetls.pfxoption (oven-sh/bun#41958, oven-sh/bun#17543), so a PKCS#12 certificate configured throughauthOptionswas never presented and the request went out uncertified. Itsnode:httpsimplementation does honourpfx(oven-sh/bun#14417), so those requests are routed there instead.src/Utility/BunHttpUtil.tsinstalls a fetch-compatible transport backed bynode:httpsas the requestfetcher(the same seamconventions.customFetchuses) when a PKCS#12 archive is configured on Bun. PEM material keeps using Bun'stlsfetch option, which works. The transport is built lazily, once per executor, closed indispose(), and not installed whenconventions.customFetchowns the transport, the same rules the DenoDeno.HttpClientpath follows. Whether the certificate needs it is decided once, in the executor constructor, not per request.Before Bun 1.4.0 neither transport presents a client certificate (
node:httpsignored the certificate and thecaas well; verified on 1.3.6 vs 1.4.0), soDocumentStore.initialize()now throws an actionable error there instead of failing per request. An unparseableprocess.versions.bunis not treated as an old runtime; the request path decides.RavenCommand.sendpasses anode:streamReadablebody through when the fetcher declaresacceptsNodeStreamBody, so streamed attachment uploads keep working on that path. The globalfetchand a user-suppliedcustomFetchdo not declare it, so their behaviour is unchanged.Also drops the
console.warnfromPfxCertificate.toBunTlsOptions(the method itself stays: it is public API, and the executor no longer calls it for a PKCS#12 archive on Bun) and documents the Bun runtime inREADME.md, including the pre-existing Changes API gap (Bun's built-in WebSocket ignores thewspackage's TLS options — upstream oven-sh/bun#31396, and for the PKCS#12 half oven-sh/bun#42322).Transport design notes
certificate.toSocketOptions().pfxrather thaninstanceof PfxCertificate: the package ships CommonJS and ESM builds with separate class identities, and a certificate created from the other build would silently fail theinstanceofcheck and go out uncertified. Thenode:streamReadablecheck on request bodies is duck-typed for the same reason.FormData,Blob,URLSearchParamsand web streams are encoded by the platformResponse, which also yields thecontent-type(including the multipart boundary the batch command relies onfetchto generate). Anode:streamReadableis piped, so a large attachment does not have to fit in memory first.content-length:node:httpwould otherwise fall back to chunked transfer encoding. The transport mirrorsfetchand always sends a length, including an explicitcontent-length: 0for body-less non-GET/HEAD requests. A streamed body has no length and stays chunked, exactly as undici sends it on Node.accept-encodingis built from what the runtime'szlibcan decode:gzip,deflate,br, pluszstdwherecreateZstdDecompressexists (Node >= 22.15, Bun >= 1.2; an older runtime gets the raw bytes). A decoded response dropscontent-encodingandcontent-length, asfetchdoes. Decompression runs throughstream.pipeline, so a source error tears the decompressor down instead of leaving the body hanging.Responseconstructor rejects a body for those statuses, and a gunzip on empty input is an unhandledZ_BUF_ERROR.abortlistener stays registered until the request closes, not just until the headers land, so an abort during a streamed response body tears the connection down too. The body then fails with the signal'sAbortError(RequestExecutorbranches onerror.name === "AbortError"for its timeout path) rather than anECONNRESET.BunTlsOptions.pfxstays in the type for shape compatibility with Node's TLS options; its doc now only warns that Bun'sfetchignores it.Verification run locally:
tsc --noEmitclean;test/Utility/BunHttpUtilTest.ts+test/Executor/RequestExecutorBunTests.ts38 passing across 5 consecutive runs (the transport tests no longer use timers);eslintclean on every file this PR touches;npm run check-exportspasses.npm run lintover the whole repo reports 4 errors, all in the generated, gitignoredtest/cloudflare-nitro/.nitro/types/output — unrelated to this change. There is nonpm run buildscript (prepareruns tshy);npm run check-importswas not run because it rewritessrc/index.ts. End-to-end verified on Bun 1.4.3 against RavenDB 7.2.6 over a PFX certificate (HttpsTest, and aDocumentStorewithuseHttpDecompression = truereceivingcontent-encoding: zstd).Type of change
x.y.z->x.y.z)Target branch and backports
v7.2,v7.1,v7.0,v6.0)How risky is the change?
Backward compatibility
Server compatibility
Affected runtimes
Public API
src/index.ts(npm run check-exportspasses).package.jsonandCLIENT_VERSIONinsrc/Http/RequestExecutor.ts(sync / release PRs only)Documentation update
README.mdhas been updatedTesting by Contributor
npm run lint,npm run build,npm run check-exportsandnpm run check-importspass locallyRAVENDB_TEST_SERVER_PATH/RAVENDB_SERVER_VERSION)Dependencies
Is there any existing behavior change of other features due to this change?
Two, both on Bun only:
node:httpsrather thanfetch. Previously the request went out with no client certificate and the server rejected it; it now authenticates. PEM on Bun is unchanged.DocumentStore.initialize(). Previously every request silently went out uncertified. The error names theopenssl pkcs12 -in cert.pfx -out cert.pem -nodesconversion andtype: "pem"as the way to stay on an older Bun.